Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

makefiles/toolchain: fix command -v multiple commands #10889

Merged
merged 1 commit into from
Feb 8, 2019
Merged

makefiles/toolchain: fix command -v multiple commands #10889

merged 1 commit into from
Feb 8, 2019

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Jan 28, 2019

Contribution description

command -v first second third only works in bash and not in sh.
So replace with multiple calls to command.

This fixes using objcopy when the toolchain objcopy is not available.

Testing procedure

I found this when building in docker with a machine without toolchain.

If you have your arm toolchain installed outside of the normal path you can try with the same procedure of changing the PATH.
Or use another board you do not have the toolchain for.

With this PR

It fallback to objcopy:

BOARD=samr21-xpro PATH=/usr/bin:/bin make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
/bin/sh: 1: arm-none-eabi-gcc: not found
/usr/bin/objcopy

TOOLCHAIN=llvm BOARD=samr21-xpro PATH=/usr/bin:/bin make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
/bin/sh: 1: arm-none-eabi-gcc: not found
/usr/bin/objcopy

Original behavior is kept:

BOARD=samr21-xpro  make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
/opt/gcc-arm-none-eabi-7-2018-q2-update/bin/arm-none-eabi-objcopy

TOOLCHAIN=llvm BOARD=samr21-xpro  make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
/opt/gcc-arm-none-eabi-7-2018-q2-update/bin/arm-none-eabi-objcopy

The arm-none-eabi-gcc error is unrelated to this PR and I think is related to #10850 and is also present in master.

Behavior in master

Master does not fallback to objcopy.

BOARD=samr21-xpro PATH=/usr/bin:/bin make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
/bin/sh: 1: arm-none-eabi-gcc: not found
/home/harter/work/git/RIOT/makefiles/toolchain/gnu.inc.mk:18: objcopy not found. Hex file will not be created.
true
 TOOLCHAIN=llvm BOARD=samr21-xpro PATH=/usr/bin:/bin make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
/bin/sh: 1: arm-none-eabi-gcc: not found
/home/harter/work/git/RIOT/makefiles/toolchain/llvm.inc.mk:25: objcopy not found. Hex file will not be created.
true

Normal behavior was the same

BOARD=samr21-xpro  make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
/opt/gcc-arm-none-eabi-7-2018-q2-update/bin/arm-none-eabi-objcopy
TOOLCHAIN=llvm BOARD=samr21-xpro  make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
/opt/gcc-arm-none-eabi-7-2018-q2-update/bin/arm-none-eabi-objcopy

Low level justification

Difference between bash and sh and make uses sh by default.

sh -c 'command -v this does not exist false true'
# nothing is printed

bash -c 'command -v this does not exist false true'
false
true

Issues/PRs references

I notified this in #10870

Follow ups

Other boards/cpus are defining objcopy and other toolchains in their own Makefile.include when they should be using this one. It also goes for all the compiler variables.

@cladmi
makefiles/toolchain: fix command -v multiple commands
566c990 
`command -v first second third` only works in `bash` and not in `sh`.
So replace with multiple calls to `command`.

This fixes using `objcopy` when the toolchain `objcopy` is not available.
@cladmi cladmi added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … labels Jan 28, 2019
@cladmi
Copy link
Contributor Author

cladmi commented Jan 28, 2019

Another update could be to define true as fallback anyway and do not print a warning.

@emmanuelsearch
Copy link
Member

emmanuelsearch commented Jan 29, 2019 •
edited
Loading

Tested on OS X, this is what I get, so... no change compared to master?

MacBookEmmanuel:RIOT emmanuel$ BOARD=samr21-xpro PATH=/usr/bin:/bin make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
bash: arm-none-eabi-gcc: command not found
/Users/emmanuel/Documents/work/CODING/RIOT/makefiles/toolchain/gnu.inc.mk:18: objcopy not found. Hex file will not be created.
true
MacBookEmmanuel:RIOT emmanuel$ TOOLCHAIN=llvm BOARD=samr21-xpro PATH=/usr/bin:/bin make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
bash: arm-none-eabi-gcc: command not found
/Users/emmanuel/Documents/work/CODING/RIOT/makefiles/toolchain/llvm.inc.mk:25: objcopy not found. Hex file will not be created.
true
MacBookEmmanuel:RIOT emmanuel$ BOARD=samr21-xpro  make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
/opt/arm-none-eabi-gcc-6-q2/bin/arm-none-eabi-objcopy
MacBookEmmanuel:RIOT emmanuel$ TOOLCHAIN=llvm BOARD=samr21-xpro  make --no-print-directory -C examples/hello-world/ info-debug-variable-OBJCOPY
/opt/arm-none-eabi-gcc-6-q2/bin/arm-none-eabi-objcopy

@cladmi
Copy link
Contributor Author

cladmi commented Jan 29, 2019

We checked IRL, and @emmanuelsearch does not have objcopy or gobjcopy installed. Which looks common on mac according to

RIOT/boards/native/Makefile.include

Lines 28 to 33 in 782b181

ifeq (0,$(shell which gobjcopy 2>&1 > /dev/null ; echo $$?))
export OBJCOPY ?= gobjcopy
else
# If gobjcopy is not available, just do nothing. The hexfile
# is not used for native anyways.
export OBJCOPY ?= true

When replacing the last objcopy by objdump for example, we correctly fallback to objdump.
So this PR allows to fallback on mac. Did not tested the behavior on master.

smlng
smlng approved these changes Feb 4, 2019
View reviewed changes
Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested, works on macOS too (just to be sure).

Side node: looks like some tools should be queried for in a more central part of the build system than twice (the same) for gcc and llvm.

@smlng
Copy link
Member

smlng commented Feb 4, 2019

btw. I have gobjcopy and other installed

@cladmi cladmi mentioned this pull request Feb 5, 2019
Closed
19 tasks
@jcarrano jcarrano merged commit 43172e2 into RIOT-OS:master Feb 8, 2019
@cladmi cladmi deleted the pr/objcopy/fallback branch February 11, 2019 15:51
@jcarrano jcarrano mentioned this pull request Feb 11, 2019
Closed
@cladmi
Copy link
Contributor Author

cladmi commented Feb 11, 2019

We noticed with @jcarrano while discussing this, that the case from master was working on his machine as on ArchLinux SHELL=/bin/bash by default in Make.

@aabadie aabadie added this to the Release 2019.04 milestone Feb 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smlng smlng smlng approved these changes

Assignees
No one assigned
Labels
Area: build system Area: Build system Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2019.04
Development

Successfully merging this pull request may close these issues.
5 participants
@cladmi @emmanuelsearch @smlng @aabadie @jcarrano